Add path.data functionality #133
Conversation
But maintain reverse compatibility for older versions of Logstash I don't yet know how to test this...
I really need to stop thinking in Python...
| @@ -170,6 +171,7 @@ def register | |||
| require "digest/md5" | |||
| @logger.info("Registering file input", :path => @path) | |||
| @host = Socket.gethostname.force_encoding(Encoding::UTF_8) | |||
| @settings = defined?(LogStash::SETTINGS) ? LogStash::SETTINGS : nil | |||
There was a problem hiding this comment.
Can you add a comment above this line that says this is specifically for checking if we're on Logstash 5.x?
Also, @settings probably doesn't need to be an instance variable, does it?
And document what settings is for, and why.
|
LGTM other than having no tests. Thoughts? |
I'm wracking my brain trying to figure out a way to test this, either with integration or unit tests. I'm coming up blank. I think my brain is tired. |
|
@untergeek agreed. I'm OK merging this as-is if we open a ticket to add an integration test for sincedb default locations. Sound ok? |
You agree that my brain is tired? 😛 I'll add a ticket to figure out how to test sincedb locations. |
Maaaybe my brain is also tired and is seeking a nap ;P |
|
LGTM |
I really need to stop thinking in Python... Fixes #133
And document what settings is for, and why. Fixes #133
...but maintain reverse compatibility for older versions of Logstash
I don't yet know how to test this...